Skip to content

fix(rewrite): skip cat rewrite when incompatible flags present#847

Merged
FlorianBruniaux merged 1 commit intortk-ai:developfrom
MauroDeryckere:fix/cat-flag-rewrite
Mar 28, 2026
Merged

fix(rewrite): skip cat rewrite when incompatible flags present#847
FlorianBruniaux merged 1 commit intortk-ai:developfrom
MauroDeryckere:fix/cat-flag-rewrite

Conversation

@MauroDeryckere
Copy link
Copy Markdown

Summary

  • Skip catrtk read rewrite when cat is invoked with flags that have different semantics than rtk read (e.g. -A, -v, -e, -t, -s, --show-all)
  • Allow rewrite for cat -n since it maps correctly to rtk read -n (both mean line numbers)
  • Plain cat file continues to rewrite to rtk read file as before

Problem

rtk rewrite blindly forwards all cat flags to rtk read. This causes incorrect behavior because the flags have different semantics:

Flag cat meaning rtk read meaning
-n Line numbers Line numbers (compatible)
-v Show non-printing chars Verbosity level (incompatible)
-A Show all (non-printing + endings) Not supported (fails)
-e Show non-printing + $ at EOL Not supported
-t Show non-printing + tabs as ^I Not supported
-s Squeeze blank lines Not supported

For example, cat -A file.cpp was rewritten to rtk read -A file.cpp which fails with rtk: Failed to resolve 'read' via PATH.

Fix

Early check in rewrite_segment(): if the command starts with cat and the first argument starts with - (but is not -n), return None to skip the rewrite. This follows the same pattern used for head -N and tail special cases.

Test plan

  • New test test_rewrite_cat_with_incompatible_flags_skipped — verifies -A, -v, -e, -t, -s, --show-all all return None
  • New test test_rewrite_cat_with_compatible_flags — verifies cat -n file.txt rewrites to rtk read -n file.txt
  • Existing test_rewrite_cat_file still passes — plain cat filertk read file
  • Full test suite: 1119 passed, 3 ignored

@pszymkowiak pszymkowiak added bug Something isn't working effort-small Quelques heures, 1 fichier labels Mar 26, 2026
@pszymkowiak
Copy link
Copy Markdown
Collaborator

[w] wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type bug-fix
🟢 Risk low

Summary

Fixes the cat-to-rtk-read rewrite to skip commands with incompatible flags (e.g., -A, -v, -e, -t, -s, --show-all) that have different semantics in rtk read, while still allowing the compatible -n flag. Previously, cat with unsupported flags was blindly rewritten, causing rtk read failures.

Review Checklist

  • Tests present
  • Breaking change
  • Docs updated

Analyzed automatically by wshm · This is an automated analysis, not a human review.

@aeppling
Copy link
Copy Markdown
Contributor

Hey

We are cleaning up the codebase and improving the project structure for better onboarding. As part of this effort, PR #826 reorganizes src/ from a flat layout into subfolders.

No logic changes — only file moves and import path updates.

What you need to do

Rebase your branch on develop when receiving this comment:

git fetch origin && git rebase origin/develop

Git detects renames automatically. If you get import conflicts, update the paths:

use crate::git;        // now: use crate::cmds::git::git;
use crate::tracking;   // now: use crate::core::tracking;
use crate::config;     // now: use crate::core::config;
use crate::init;       // now: use crate::hooks::init;
use crate::gain;       // now: use crate::analytics::gain;

Need help rebasing? Tag @aeppling

cat flags like -v, -A, -e, -t, -s have different semantics than
rtk read flags (-v means verbose, -n means line numbers). Blindly
forwarding flags caused incorrect behavior (e.g. cat -A file →
rtk read -A file which fails).

Only skip rewrite for incompatible flags. cat -n (line numbers) maps
correctly to rtk read -n, so it is still rewritten for token savings.
Plain `cat file` continues to rewrite to `rtk read file` as before.
@FlorianBruniaux FlorianBruniaux merged commit 9b9075c into rtk-ai:develop Mar 28, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working effort-small Quelques heures, 1 fichier

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants